fix(hooks): detect grep/read shell commands in Codex exec_command#1431
Open
octo-patch wants to merge 1 commit intooraios:mainfrom
Open
fix(hooks): detect grep/read shell commands in Codex exec_command#1431octo-patch wants to merge 1 commit intooraios:mainfrom
octo-patch wants to merge 1 commit intooraios:mainfrom
Conversation
oraios#1394) When the Codex client sends shell work via `exec_command`, the tool name is always `exec_command` and the actual command is under `tool_input.cmd`. The previous `is_grep_tool` / `is_read_file_tool` checks inspected only the tool name and therefore never fired for Codex grep/read workflows. Add `_GREP_SHELL_COMMANDS` and `_READ_SHELL_COMMANDS` frozensets and a helper `_get_exec_command_name()` that extracts the basename of the first token in `cmd`. Both detection methods now use these to classify `exec_command` calls when the client is `HookClient.CODEX`, leaving all other client paths unchanged. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
Contributor
|
Thanks for the PR. We don't need the new tests, the current logic is very simple. Pls remove them, format the code and add an entry to the changelog. Then this can be merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1394
Problem
serena-hooks remind --client=codexnever counted Codex grep/read workflows because the Codex client sends all shell work asexec_command(with the actual command undertool_input.cmd). The previous detection checked only the tool name string, sois_grep_tool()andis_read_file_tool()always returnedFalsefor Codex — leaving the remind hook entirely blind to Codex exploration patterns.Solution
_GREP_SHELL_COMMANDSand_READ_SHELL_COMMANDSfrozensets listing common grep-like (grep,rg,ag,ack,fgrep,egrep) and read-file-like (cat,head,tail,sed,less,more,bat) shell commands._get_exec_command_name()helper that extracts the basename of the first token fromtool_input.cmd, handling both bare names (rg) and path-prefixed invocations (/usr/bin/grep).is_grep_tool()andis_read_file_tool()to use these when the client isHookClient.CODEXand the tool name isexec_command; all other client paths are unchanged.Testing
Added
TestCodexExecCommandDetectionclass with 13 tests covering:rg,grep, path-prefixedgrep,ag→is_grep_tool()sed,cat,head,tail→is_read_file_tool()ls,echo→ neitherexec_commandtools for Codex still fall through to substring matchingexec_command+rg/sedcalls trip the deny after thresholdAll 13 new tests pass. The 2 pre-existing test failures (
test_outputs_activation_message,test_activate_command) were already present onmaindue to an unrelated activation message text change.